Test build #63634 has finished for PR 14608 at commit 1ae7a3b.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

I found the SQL generation for expression pretty weird with parentheses.

The current design adds parentheses at the outside of each expression. For example, "1+2" becomes "(1+2)". There was this subtle bug in UnaryMinus for negative numeric literal, as the generated SQL becomes "--number", which is a comment in SQL. I fixed it here by injecting a space.

It might be better to change these to put parentheses in the inside, e.g.

class Add {
  def sql = wrap(left) + " + " + wrap(right) 
}

class UnaryMinus {
  def sql = s"-($child.sql)"
}

def wrap(node) = if node is Literal node.sql else s"(${node.sql})"

@cloud-fan, @hvanhovell, @liancheng what do you think?

Test build #63644 has finished for PR 14608 at commit 154abba.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

adding a space LGTM. I think UnaryMinus is the only special case? Adding parentheses at the outside of each expression should work for all other cases.

Test build #63652 has finished for PR 14608 at commit 908253b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

LGTM

Shouldn't we wrap literals with () to be safe?

I'm going to merge this one in master/2.0.